Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UI bug in ATT&CK Report #949

Merged
merged 1 commit into from
Jan 30, 2021

Conversation

shreyamalviya
Copy link
Contributor

Fixes #948

attack-report-bug-fix.mp4

@codecov
Copy link

codecov bot commented Jan 29, 2021

Codecov Report

Merging #949 (32b116c) into develop (c537106) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #949   +/-   ##
========================================
  Coverage    19.07%   19.07%           
========================================
  Files          338      338           
  Lines        11482    11482           
========================================
  Hits          2190     2190           
  Misses        9292     9292           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c537106...32b116c. Read the comment docs.

@@ -159,7 +159,9 @@ class AttackReport extends React.Component {
}
// modify techniques' messages
for (const tech_id in techniques){
techniques[tech_id]['message'] = <div dangerouslySetInnerHTML={{__html: marked(techniques[tech_id]['message'])}} />;
if (typeof techniques[tech_id]['message'] === 'string') {
Copy link
Collaborator

@mssalvatore mssalvatore Jan 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we understand why the messages are objects instead of strings? What is the root cause of this issue? Are we just side-stepping it by checking the type?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

marked() modifies them in place. We need to change this behaviour(maybe there's a param for this? maybe we pass a copy of the string instead?)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VakarisZ Does marked() modify in place? All of the usage I can find implies that it returns a value, and javascript strings are immutable. https://marked.js.org/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think you mistyped and meant that modifyTechniqueData() modifies in-place. I think we're on the same page.

@mssalvatore mssalvatore merged commit c8c763d into guardicore:develop Jan 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI bug in attack report
3 participants